Skip to content

Remove the disused selectLimit metadata query - #462

Merged
tonygermano merged 1 commit into
OpenIntegrationEngine:mainfrom
mgaffigan:feat/drop-select-limit
Nov 9, 2026
Merged

tonygermano merged 1 commit into
OpenIntegrationEngine:mainfrom
mgaffigan:feat/drop-select-limit

Conversation

@mgaffigan

@mgaffigan mgaffigan commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Dead since the table metadata probe switched to WHERE 1 = 0. Dropped the REST parameter, the Drivers dialog column, and the dbdrivers.xml attribute.

DriverInfo.selectLimit is retained as a deprecated no-op: the field still has to exist for previously saved driver lists to deserialize, and the constructors keep outside callers building. It no longer participates in equals, so the dialog does not report a phantom change when loading an old config.

Parameter can be removed without breaking users - extra parameters are ignored. I can't find any plugins that use the call directly, and it doesn't seem likely that one would, so that parameter was dropped without a deprecated overload.

Closes #461

Release note

The deprecated selectLimit parameter has been removed from POST /connectors/jdbc/_getTables and DatabaseConnectorServletInterface.getTables. Java code calling getTables through client-core must drop that argument. Custom "Select with Limit Query" values are no longer shown in the Database Drivers settings or read from dbdrivers.xml.

@github-actions

github-actions Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Test Results

  141 files  + 16    141 suites  +16   3m 27s ⏱️ + 1m 35s
  919 tests +208    919 ✅ +208  0 💤 ±0  0 ❌ ±0 
1 051 runs  +298  1 045 ✅ +298  6 💤 ±0  0 ❌ ±0 

Results for commit fdff780. ± Comparison against base commit 9d99f43.

♻️ This comment has been updated with latest results.

@gibson9583 gibson9583 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please address items I left in line

@mgaffigan

mgaffigan commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor Author

@gibson9583, I don't see a clean way to leave a @deprecated client method - the point of the PR was to address the complaint that the param was left present. Client does not allow overloads and pass-throughs, so this would be a new method plumbed top-to-bottom.

@pacmano1 / @tonygermano, you were the original requestor on selectLimit being removed. Can you comment? I'm fine with the status quo in main.

@NicoPiel NicoPiel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

non-blocking:

  • Plugins built against the old interface: removes this parameter on purpose, and the server was already ignoring it. A third-party plugin would only break if it called getTables itself, which is unlikely. A line in the release notes covers it.
  • new DriverInfo(a, b, c, null) no longer compiling: nothing in the repo calls it that way. Outside code would get a clear compile error, not a runtime failure, and casting the null fixes it.

@tonygermano

Copy link
Copy Markdown
Member

@gibson9583, I don't see a clean way to leave a @deprecated client method - the point of the PR was to address the complaint that the param was left present. Client does not allow overloads and pass-throughs, so this would be a new method plumbed top-to-bottom.

@pacmano1 / @tonygermano, you were the original requestor on selectLimit being removed. Can you comment? I'm fine with the status quo in main.

@mgaffigan What I had intended with my original suggestion was something like

// original api method with selectLimit
@deprecated
public SortedSet<Table> getTables(String channelId, String channelName, String driver,
    String url, String username, String password, Set<String> tableNamePatterns, String selectLimit,
    Set<String> resourceIds) {
  return getTables(channelId, channelName, driver, url, username, password, tableNamePatterns, resourceIds);
}

// new api method without selectLimit
public SortedSet<Table> getTables(String channelId, String channelName, String driver,
    String url, String username, String password, Set<String> tableNamePatterns, Set<String> resourceIds) {
  // implementation without selectLimit
}

Does your response to @gibson9583 say this is not possible with the client?

@mgaffigan

Copy link
Copy Markdown
Contributor Author

Does your response to @gibson9583 say this is not possible with the client?

Yes. That's what I went for originally, but I ran into problems with the RPC magic.

@tonygermano tonygermano left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gibson9583, a deprecated getTables overload would need changes to the client proxy plumbing that every servlet interface uses (Client.getServlet rejects methods without JAX-RS annotations), just to keep an already-ignored parameter alive. Instead, 4.7.0 keeps the parameter but ignores it, and its release notes announce that it's deprecated and will be removed in an upcoming release (#503). This PR completes that cycle.

Older client-core jars keep working against newer servers, since the server ignores the extra query parameter. The new jar works against older servers too. The break is limited to upgrading the jar, which is a one-argument compile error.

@mgaffigan, I added a "Release note" section to the PR description announcing the removal. Feel free to adjust the wording.


Drafted with Claude Code

@gibson9583
gibson9583 dismissed their stale review November 8, 2026 19:35

Dismissed at the review author’s request.

Dead since the table metadata probe switched to `WHERE 1 = 0`.  Dropped
the REST parameter, the Drivers dialog column, and the dbdrivers.xml
attribute.

`DriverInfo.selectLimit` is retained as a deprecated no-op: the field
still has to exist for previously saved driver lists to deserialize, and
the constructors keep outside callers building.  It no longer
participates in equals, so the dialog does not report a phantom change
when loading an old config.

Signed-off-by: Mitch Gaffigan <mitch.gaffigan@comcast.net>
@tonygermano
tonygermano force-pushed the feat/drop-select-limit branch from cc5f17a to fdff780 Compare November 9, 2026 21:30
@tonygermano
tonygermano merged commit fdff780 into OpenIntegrationEngine:main Nov 9, 2026
20 of 21 checks passed
@tonygermano tonygermano added this to the Next Release milestone Nov 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CLEANUP] Deprecate method signature in DatabaseConnectorServlet.getTables

5 participants